Expand main content area to viewport when zoomed out #59512
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +183 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
ellatrix
commented
Mar 1, 2024
| main { | ||
| flex: 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
@jasmussen @MaggieCabrera Sorry to ping you, but maybe with your CSS wizardry skills you can think of ways to make this work for all themes. Otherwise I think we'll have to rely on themes to add it. Or maybe this is fine for block-based themes if they don't use a custom stylesheet?
There was a problem hiding this comment.
Can we make this css conditional to having no patterns inserted instead? We only care about this when the page is empty, otherwise if we insert a pattern that doesn't fill the whole viewport height we are introducing fake space that is not really there in the frontend.
There was a problem hiding this comment.
Sorry I'm catching up after a bit. What does this property do?
I'd agree we want to ideally avoid a theme needing to add anything at all in order for the zoom prop to work.
There was a problem hiding this comment.
How does it break themes? Doesn't this CSS be just in editor? Also what makes it work in zoom out mode only?
There was a problem hiding this comment.
I think there's some misunderstandings:
- No, this CSS is not required for zoom out mode, it's just nice that the footer is pushed down when the viewport is made higher.
- Since it's not a requirement for zoom out mode, it something that we let theme implement. I think this is a nice thing on the front-end as well (pushing the footer down when there's no or little content).
- I don't see this as fake space at all, I just see this ensuring the footer is at the bottom of the page. Again, if it's something that the theme does it would be on the front-end too.
- I didn't say it breaks themes, I just said it may clash with theme CSS, but actually this may not be the case at all for block based themes 🤔
Or maybe we can rely on block-based themes never styling is-root-container, in which case this CSS may be safe to use as it won't clash with anything. It still relies on header, main and footer being top-level blocks though.
There was a problem hiding this comment.
@jasmussen when zooming out the content is scaled. We would want that the scaling of an empty page to not cause a small zoomed out patch containing header a small nothing and a footer but instead when zooming out to enlarge vertically the shape to obtain header a large nothing footer. The purpose of this trickery is to make it obvious that the big nothing is where the assembly of patterns will happen (assuming an empty page is zoomed out for building it).
There was a problem hiding this comment.
Valid enough, I think there's some nuance to explore there in terms of what that means for the blue pattern insertion plus buttons as well, and where they show up. I.e. to your point, we certainly don't want those to stack right next to each other. But we also want to avoid an accordion behavior where the document is always at least full-viewport-height when empty, and then it collapses as soon as you add a single section pattern. So I'd be tempted to apply a min-height instead of flex magic.
There was a problem hiding this comment.
@jasmussen I think this does exactly what you say: #59512 (review)
There was a problem hiding this comment.
Ideally the shape of the page doesn't change when you zoom out. If the page is short, it should be even shorter when zoomed out.
The whole point of this PR is to not have that. Maybe @richtabor can explain this better :)
There was a problem hiding this comment.
Just to clarify, this simply ensures the footer is at the bottom of an empty page, or rather when there is not enough content to push it there—and only when zoom-out mode is engaged. It helps align the interface with expectations of where content (i.e. patterns) will be inserted.
This is not a "fit" view, but rather an initial state when there's not a full page of content.
| Before | After |
|---|---|
 |
 |
draganescu
previously requested changes
Mar 6, 2024
Could you elaborate a bit more? Do you mean after the resizing? That's because the height is not being adjusted on resize, but we could add a resize handler. |
Perhaps as a follow-up; I don't think this is a blocker. |
|
I'll fix the resizing in this PR, it's just adding a resize handler and recalculate. |
Let's hone-in on the zoom-out mode of the editor, not the front-end. |
richtabor
changed the title
richtabor
approved these changes
Mar 7, 2024
|
@ellatrix mostly I meant the weirdness at 00:29 where the footer looks like missing its height suddenly after the height has to increase beyond 100vh. May be just a glitch tho. Yes to the resize handler tho 💯 |
|
Yes that should be fixed now, I had pushed a commit |
richtabor
changed the title
|
@draganescu mind giving this another review? thanks! |
|
I found a bug which I admit seems exotic:
In this video I have to try three times, in Safari, and click zoom out quickly enough: zoom-to-fit.mp4 |
|
Yea that seems a bit of an edge case. I'm ok merging without that optimization, perhaps looking into it later. |
draganescu
force-pushed
the
try/zoom-out-expand-main
branch
from
f134312 to
83dd2dd
Compare
|
Flaky tests detected in 83dd2dd. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8298001828
|
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
This was referenced Jul 15, 2024
This was referenced Oct 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
@richtabor asked if this was possible, so here it is.
Why?
I think the primary reason is to make the drop target for patterns larger, but it also makes the site look better when empty (just header/footer).
How?
bodyelement because it must be an element with the scaled element (htmlelement). We can do this for any theme I think.Testing Instructions
Enable the zoom-out mode experiment.
Create an empty page in the site editor (for a block based theme).
Click the zoom-out mode button in the top toolbar.
Testing Instructions for Keyboard
Screenshots or screencast